Skip to content

fix(security): remove localhost CORS origin, consolidate CORS in proxy#4658

Open
waleedlatif1 wants to merge 1 commit into
stagingfrom
waleedlatif1/csp-localhost-review
Open

fix(security): remove localhost CORS origin, consolidate CORS in proxy#4658
waleedlatif1 wants to merge 1 commit into
stagingfrom
waleedlatif1/csp-localhost-review

Conversation

@waleedlatif1
Copy link
Copy Markdown
Collaborator

Summary

  • Move all /api/* CORS handling from next.config.ts to proxy.ts so allowed origin is resolved per-request at runtime instead of baked at build time (which produced Access-Control-Allow-Origin: http://localhost:3000 with credentials: true in production)
  • Per-route policy table in proxy covers auth, MCP, form, and workflow execute endpoints; OPTIONS preflight short-circuited uniformly
  • Drop NEXT_PUBLIC_APP_URL build placeholder from Dockerfile (Zod has skipValidation: true; build path doesn't read it)
  • Remove 8 dead route OPTIONS handlers and their preflight tests now that the proxy handles preflight

Type of Change

  • Bug fix

Testing

Tested manually

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@vercel
Copy link
Copy Markdown

vercel Bot commented May 19, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
docs Ready Ready Preview, Comment May 19, 2026 2:43am

Request Review

@gitguardian
Copy link
Copy Markdown

gitguardian Bot commented May 19, 2026

️✅ There are no secrets present in this pull request anymore.

If these secrets were true positive and are still valid, we highly recommend you to revoke them.
While these secrets were previously flagged, we no longer have a reference to the
specific commits where they were detected. Once a secret has been leaked into a git
repository, you should consider it compromised, even if it was deleted immediately.
Find here more information about risks.


🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.

@cursor
Copy link
Copy Markdown

cursor Bot commented May 19, 2026

PR Summary

Medium Risk
Touches security-sensitive CORS behavior and preflight handling across all API routes, which could break cross-origin clients or inadvertently widen/restrict access if policies are incorrect.

Overview
CORS handling for /api/* is centralized in proxy.ts: a per-route policy table now sets Access-Control-* headers at request time (including a uniform OPTIONS short-circuit), avoiding build-time baked origins.

This removes static CORS headers from next.config.ts, deletes several route-level OPTIONS handlers plus their preflight tests, and drops the Docker build-time NEXT_PUBLIC_APP_URL placeholder that was only needed for the previous build-time CORS approach.

Reviewed by Cursor Bugbot for commit 3a8349d. Configure here.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 19, 2026

Greptile Summary

This PR fixes a production CORS misconfiguration where NEXT_PUBLIC_APP_URL was frozen at build time (defaulting to http://localhost:3000), causing browsers to reject credentialed API requests in production. All /api/* CORS handling is moved from the static next.config.ts headers into a runtime middleware function (resolveApiCorsPolicy in proxy.ts), and the eight per-route OPTIONS handlers that were made redundant are removed.

  • proxy.ts: Adds resolveApiCorsPolicy — a route-specific CORS policy table covering auth, MCP, form, and workflow execute paths — and short-circuits preflight OPTIONS requests uniformly in middleware before they reach route handlers.
  • next.config.ts: Drops all Access-Control-* headers that were baked at build time; retains non-CORS security headers (COEP/COOP) for workflow execution routes.
  • docker/app.Dockerfile: Removes the build-time NEXT_PUBLIC_APP_URL ARG/ENV placeholder since the value is now read from the runtime environment.

Confidence Score: 3/5

The reflected-origin form route path needs Vary: Origin and the double CORS header application needs resolution before this is production-safe for form embeds.

The reflected-origin form route path sets Access-Control-Allow-Origin dynamically but omits Vary: Origin, a cache-poisoning vector for any CDN in front of the app. addCorsHeaders is also still called inside the form route handler while the middleware sets the same headers, risking duplicate Access-Control-Allow-Origin values that browsers reject.

apps/sim/proxy.ts (applyCorsHeaders, buildPreflightResponse, and the /api/form branch of resolveApiCorsPolicy) and apps/sim/app/api/form/[identifier]/route.ts (addCorsHeaders calls that now overlap with middleware)

Security Review

  • Cache-poisoning via missing Vary: Origin (apps/sim/proxy.ts, resolveApiCorsPolicy/applyCorsHeaders): The form route policy dynamically reflects the Origin request header into Access-Control-Allow-Origin, but neither the middleware CORS helper nor the preflight builder adds Vary: Origin. Shared caches can serve a response carrying Access-Control-Allow-Origin: https://evil.com to a different requester, breaking same-origin protections or unexpectedly allowing cross-origin access.

Important Files Changed

Filename Overview
apps/sim/proxy.ts New CORS policy engine added; reflects Origin for form routes without Vary: Origin (cache-poisoning risk), and returns early before security filtering/analytics for all /api/ paths.
apps/sim/next.config.ts Removes build-time CORS headers for /api/* routes, replacing them with runtime proxy logic; retains non-CORS headers (COEP/COOP) for workflow execute and other routes.
apps/sim/app/api/form/[identifier]/route.ts OPTIONS handler removed (moved to proxy); POST/GET still call addCorsHeaders internally, creating potential duplicate CORS headers alongside the new middleware layer.
docker/app.Dockerfile Drops build-time NEXT_PUBLIC_APP_URL ARG/ENV since CORS origin is now resolved at runtime from the environment variable.
apps/sim/app/api/files/utils.ts Removes createOptionsResponse helper now that preflight is handled centrally in the proxy.
apps/sim/app/api/mcp/copilot/route.ts OPTIONS handler removed; MCP copilot CORS is now handled by the proxy policy table, preserving the same wildcard-origin policy.

Sequence Diagram

sequenceDiagram
    participant Browser
    participant Proxy as proxy.ts (middleware)
    participant Handler as Route Handler

    Browser->>Proxy: "OPTIONS /api/* (preflight)"
    Proxy->>Proxy: resolveApiCorsPolicy(request)
    Proxy-->>Browser: 204 + CORS headers (short-circuit)

    Browser->>Proxy: GET/POST /api/form/[id]
    Proxy->>Proxy: resolveApiCorsPolicy reflect Origin
    Proxy->>Handler: NextResponse.next() + CORS headers
    Handler->>Handler: addCorsHeaders() still called internally
    Handler-->>Proxy: Response + CORS headers again
    Proxy-->>Browser: Merged response potential duplicate CORS headers
Loading

Reviews (1): Last reviewed commit: "fix(security): remove localhost CORS ori..." | Re-trigger Greptile

Comment thread apps/sim/proxy.ts
Comment thread apps/sim/proxy.ts
Comment thread apps/sim/proxy.ts
@waleedlatif1 waleedlatif1 changed the base branch from staging to main May 19, 2026 02:32
@waleedlatif1 waleedlatif1 changed the base branch from main to staging May 19, 2026 02:37
Move all /api/* CORS handling from next.config.ts to proxy.ts so the
runtime can resolve allowed origin per-request instead of baking it at
build time (which produced "Access-Control-Allow-Origin: http://localhost:3000"
with credentials:true in production).

- proxy.ts: per-route CORS policy table covering auth, MCP, form, and
  workflow execute endpoints; OPTIONS preflight short-circuit; Vary:
  Origin when origin is not '*'; form routes defer to route handler's
  addCorsHeaders to avoid double-setting
- next.config.ts: drop all /api/* Access-Control-Allow-* headers; keep
  COEP/COOP/CSP
- deployment.ts: addCorsHeaders sets Vary: Origin alongside reflected
  Allow-Origin
- Dockerfile: drop NEXT_PUBLIC_APP_URL build placeholder (Zod has
  skipValidation:true; build path doesn't read it)
- Remove 8 dead OPTIONS handlers and their preflight tests now that the
  proxy handles preflight uniformly
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant